Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not log request details for rewards external wallet session endpoints #8767

Merged
merged 1 commit into from
May 12, 2021
Merged

Do not log request details for rewards external wallet session endpoints #8767

merged 1 commit into from
May 12, 2021

Conversation

zenparsing
Copy link
Collaborator

Resolves brave/brave-browser#15661

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  • Start browser with a clean profile and enable "Rewards Verbose Logging" at brave://flags
  • Relaunch the browser
  • Connect an Uphold wallet
  • Navigate to brave://rewards-internals and view the logs.
  • Verify that a request for "*.uphold.com/oauth2/token" does not appear in the logs.

@zenparsing zenparsing requested a review from emerick May 10, 2021 20:18
@zenparsing zenparsing requested a review from a team as a code owner May 10, 2021 20:18
@zenparsing zenparsing self-assigned this May 10, 2021
emerick
emerick approved these changes May 10, 2021
View reviewed changes
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zenparsing
Copy link
Collaborator Author

zenparsing commented May 12, 2021 •
edited
Loading

CI

  • post-init failed on audit-deps (unrelated)
  • linux failed on
    • RewardsPublisherBrowserTest.VisitVerifiedPublisher (unrelated)
    • BraveAdsBrowserTest/BraveAdsUpgradeBrowserTest.UpgradePath (unrelated)
  • macos failed on AdBlockServiceTest.CnameCloakedRequestsGetBlocked (unrelated)

@zenparsing zenparsing merged commit 813b718 into master May 12, 2021
@zenparsing zenparsing deleted the ksmith-hide-oauth-request branch May 12, 2021 17:51
@zenparsing zenparsing added this to the 1.26.x - Nightly milestone May 12, 2021
zenparsing pushed a commit that referenced this pull request May 12, 2021
@brave-browser-releases @zenparsing
Uplift of #8598 and #8767 (squashed) to beta
a30d634
@zenparsing zenparsing mentioned this pull request May 12, 2021
Merged
7 tasks
@kjozwiak
Copy link
Member

Verification PASSED on Win 10 x64 using the following build:

Brave | 1.26.21 Chromium: 91.0.4472.57 (Official Build) nightly (64-bit)
-- | --
Revision | e3443317fa07f1e9997e4a9c738eddfefc3c0292-refs/branch-heads/4472_54@{#6}
OS | Windows 10 OS Version 2009 (Build 19042.964)
  • verified the STR outlined via Do not log request details for rewards external wallet session endpoints #8767 (comment)
  • ensured that clicking on Refresh correctly populates the empty text field with the log
  • ensured that clicking on Download log correctly initiates the download process/flow
  • ensured that *.uphold.com/oauth2/token didn't appear anywhere within the log after connecting to an Uphold account
    • also searched for oauth2 & oauth2/token within the log as a double check

Example:

logRewards

@kjozwiak kjozwiak mentioned this pull request May 18, 2021
Closed
@kjozwiak
Copy link
Member

Created brave/brave-browser#15924 as a follow up as #brave-rewards-verbose-logging is missing from Android.

@stephendonner stephendonner mentioned this pull request May 19, 2021
Closed
@kjozwiak kjozwiak mentioned this pull request May 22, 2021
Merged
24 tasks
@kjozwiak
Copy link
Member

Went through the above cases on Android and added the verification notes on Nightly for uplift via #8841 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emerick emerick emerick approved these changes

@deeppandya deeppandya Awaiting requested review from deeppandya

Assignees

@zenparsing zenparsing

Labels
None yet
Projects
None yet
Milestone
1.26.x - Release
Development

Successfully merging this pull request may close these issues.

Remove request body details from Rewards verbose logging
3 participants
@zenparsing @kjozwiak @emerick